-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sdtrig support #486
Sdtrig support #486
Conversation
Thanks for starting to think about debug extensions. Could you say what the plans are for developing |
Yes I am currently working on it thats why its a draft PR. I am trying to complete this ASAP. Will keep you posted about progress. |
…ssue#487) Solving lto wrapper warning (issue#487)
change warning check in RepeatReadTest
Hi @billmcspadden-riscv, this PR is failing the CI for breakpoint test in riscv test. The way I see it, when the sdtrig registers were absent, the access would just be ignored as there was no register at tdata1 and go on. but as now the register is present and is writable in debug mode only (which is still not present in SAIL-RISCV), so it gives illegal instruction exception. |
Either the test is wrong or your implementation is wrong. Which is it? |
At a guess, the test is now wrong, because the expected outcome is "access failure" because the test assumes the register is not there. That has changed. So either remove the failing tests, or change their expected outcome. @Mudassir10X Could you show us the source of the failing tests? |
Is this the problem with the new definition of what happens when you access
an unimplemented CSR (that it basically becomes undefined).]?
From an architectural test point of view, that means that if you can't
discover whether a CSR exists (either programmatically e.g.
via something like unified discovery, or passing arguments into the test -
which will eventually be the riscv-config)
then you have to skip testing the feature. Changing the test won't work.
If Sail assumes that this CSR is not present, and is now optionally
present, and we have to way to test whether it is present,
then it is untestable.
…On Fri, Jun 7, 2024 at 11:51 PM Martin Berger ***@***.***> wrote:
Either the test is wrong or your implementation is wrong. Which is it?
At a guess, the test is now wrong, because the expected outcome is that
the expected outcome is an access failure because the test assumes the
register is not there. That has changed. So either remove those 4 tests, or
change the expected outcome.
—
Reply to this email directly, view it on GitHub
<#486 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJQOYX3S5IUA23546STZGKSWFAVCNFSM6AAAAABIFEZBA6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVHA2DENBSGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
There can be 2 ways to solve this problem.
PS: From ACT point of view, option 2 is viable because if a CSR is not defined then you should not test any feature related to that CSR. But this case in more interesting one. In this case, a CSR (tdata1) has been defined but it is not accessible as Debug mode is not available in Sail, so now there is a CSR whose definition and mapping is available in Sail but you cant read/write it in a test because there is no legal way to access it without Debug mode. In this case, option 1 can be chosen as a short term solution. |
I want to echo what Umer said: there is a difference between an
unimplemented CSR and an inaccessible CSR.
The spec used to say that accessing unimplemented CSR would cause an
illegal instruction trap - but that has been loosened to say that the
result is Reserved.
That has two implications:
- tests must be passed a variable that indicates whether a CSR is
implemented or not
(as a command line option, extracted from the riscv-config YAML file)
- tests that access a CSR must be conditioned on that variable as part of
the TEST_CASE macro.
It is still just fine to test accessbility (and we should) under conditions
that explore both allowed and disallowed access
( which could be a function of priv mode, or by type of access (read or
write), or a CSR enable/disable bit)
…On Tue, Jun 11, 2024 at 4:29 AM Umer Shahid ***@***.***> wrote:
There can be 2 ways to solve this problem.
1. If the test is just checking if some CSR is defined or not, and the
passing criteria is "access failure", then test is needed to be modified to
change the passing criteria. The test should be considered "pass" if CSR is
not defined and giving "access failure" or if CSR is defined then legal
read/write should be considered as "pass".
2. Instead of checking if a certain CSR is defined or not, a test
should check the valid functionality of that CSR and a flag should be added
which chooses the test to be run based on the architecture state (or
riscv-config string). Test will be selected only if a certain CSR is
defined and accessible but if the CSR is undefined then that test should
not be selected.
PS: From ACT point of view, option 2 is viable because if a CSR is not
defined then you should not test any feature related to that CSR. But this
case in more interesting one. In this case, a CSR (tdata1) has been defined
but it is not accessible as Debug mode is not available in Sail, so now
there is a CSR whose definition and mapping is available in Sail but you
cant read/write it in a test because there is no legal way to access it
without Debug mode. In this case, option 1 can be chosen as a short term
solution.
—
Reply to this email directly, view it on GitHub
<#486 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJQQRX6H3F4IDXRAXRTZG3NQPAVCNFSM6AAAAABIFEZBA6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRQGUYDQMZTGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@Mudassir10X can you please close this one, since new PR has been added. |
This PR has been replaced by PR #573 |
This PR is for the support of sdtrig extension as proof of concept for triggers support in SAIL-RISCV. It is still incomplete and work is being actively done. reviewers and contributors are welcomed.
This PR is related to this issue in RISCV dev partners.